feat(spec-specs, tests): merge EIP-8037 to forks/amsterdam#2901
feat(spec-specs, tests): merge EIP-8037 to forks/amsterdam#2901spencer-tb wants to merge 114 commits into
forks/amsterdam#2901Conversation
…se (ethereum#2363) * feat(spec-specs): update EIP-8037 to match latest spec revision * feat(tests): add EIP-8037 state creation gas cost increase tests
Co-authored-by: Ben Adams <thundercat@illyriad.co.uk>
Add sstore_state_gas(), code_deposit_state_gas(), and create_state_gas() calculator methods to Fork. Replace Spec constants and manual gas arithmetic across all EIP-8037 tests with dynamic fork method calls. Remove redundant Op.STOP, hardcoded numbers from docstrings, and add fork: Fork parameter to all test functions that use fork methods.
Test CREATE with max initcode size using proper regular/state gas split via the reservoir. Verifies gas boundary behavior with EIP-8037 two-dimensional metering.
Align EIP-8037 gas constant references with upstream renames: - GAS_STORAGE_UPDATE → GAS_COLD_STORAGE_WRITE - GAS_COLD_SLOAD → GAS_COLD_STORAGE_ACCESS - GAS_WARM_ACCOUNT_ACCESS → GAS_WARM_ACCESS Bump gas_limit on tests added to upstream after EIP-8037 branched, which now need extra gas for EIP-8037 state gas costs.
… format runs in withdrawal request contract tests (ethereum#2532) * fix(tests): prevent tx_gas_limit double-accumulation across fixture format runs in withdrawal request contract tests * fix: mypy * fix: ruff * fix: ruff * chore(tests): refactor fix to fork-aware transactions to prevent mutation * chore(test): add a warning to all tests that could mutate vars; address in later PR Add a lightweight repr-based snapshot hook to the filler plugin that warns whenever any ``pytest.param`` value is mutated during a test run. A subsequent PR could address this by returning values instead of mutating, then flipping the hook to a hard failure. --------- Co-authored-by: fselmo <fselmo2@gmail.com>
…hereum#2526) Co-authored-by: fselmo <fselmo2@gmail.com>
… gas validity test (ethereum#2583) Co-authored-by: Stefan <22667037+qu0b@users.noreply.github.com>
Conditionally increase tx gas_limit (and env gas_limit where needed) when fork >= Amsterdam to account for EIP-8037 state creation gas costs. 137 files, 9 with env gas_limit bumps. Headroom: 2,000,000.
Move MAX_CODE_SIZE check before gas charges, charge keccak hash cost (regular gas) before code deposit state gas, and add tests for over-max code size and reservoir preservation after OOG.
On CREATE/CREATE2 address collision the 63/64 gas allocation is burned but was not added to regular_gas_used, leaving it invisible to 2D block gas accounting. Per EIP-684 collision behaves as an immediate exceptional halt, so the burned gas belongs in the regular dimension.
…and subcall pattern Co-authored-by: Mario Vega <marioevz@gmail.com>
The static test skip list and conftest were a temporary workaround for EIP-8037 gas failures. The ported static tests in tests/ported_static/ replace this approach; failures are tracked in ethereum#2601.
…thereum#2603) * fix(execute): use --sender-fund-refund-gas-limit for all funding txs On EIP-8037 networks, simple value transfers to new accounts require more than 21000 gas due to GAS_NEW_ACCOUNT state gas (112 * cpsb). The default Transaction gas_limit of 21000 causes 'intrinsic gas too low' errors during test setup. Changes: - contracts.py: Use 200000 gas for deterministic factory deployer funding - pre_alloc.py: Pass sender_fund_refund_gas_limit to Alloc and use it for simple EOA funding transactions Usage: --sender-fund-refund-gas-limit 200000 * chore: additional fixes for execute remote funds w/ higher gas limits * fix: bump all gas limits to 200k for EIP-8037 state creation costs - sender.py: bump --sender-fund-refund-gas-limit default 21000 → 200000 - pre_alloc.py: bump funding_gas_limit default 21000 → 200000 - pre_alloc.py: use configurable gas limit for per-test refund txs - execute_recover.py: bump recovery refund gas limit 21000 → 200000 EIP-8037 charges GAS_NEW_ACCOUNT (112 × cost_per_state_byte = 131488) for transfers to new accounts, making 21000 gas insufficient for all funding, refund, and recovery transactions. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Felipe Selmo <fselmo2@gmail.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… gas (ethereum#2595) Co-authored-by: spencer-tb <spencer.tb@ethereum.org>
…um#2610) Co-authored-by: spencer-tb <spencer.tb@ethereum.org>
…ode size validation (ethereum#2608) * fix(spec): charge CREATE state gas after initcode size validation Move charge_state_gas(STATE_BYTES_PER_NEW_ACCOUNT) from create()/create2() into generic_create(), after the MAX_INIT_CODE_SIZE check. Previously, state gas was charged before the initcode size check, so a CREATE with oversized initcode would persist state_gas_used equal to the account creation state gas cost (STATE_BYTES_PER_NEW_ACCOUNT * cost_per_state_byte) even though no account was ever created and no state was touched. Reported by @AskDragan (reth): ethereum#2578 (comment) * fix(spec): check static context before gas in CREATE/CREATE2 Move the is_static check from generic_create() into create() and create2(), before stack pops and charge_gas(). This is consistent with SSTORE, CALL, and SELFDESTRUCT which all check static context before any gas charging.
… the correct token formula
…evel failure (ethereum#2689) Co-authored-by: spencer-tb <spencer.tb@ethereum.org>
|
|
kclowes
left a comment
There was a problem hiding this comment.
Got through Osaka, Berlin, and Byzantium today. I saw a few things that need to be updated, but nothing major!
Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com>
…e pattern Co-authored-by: marioevz <11726710+marioevz@users.noreply.github.com>
Co-authored-by: marioevz <11726710+marioevz@users.noreply.github.com>
…r 8037 Co-authored-by: marioevz <11726710+marioevz@users.noreply.github.com>
… 8037 Co-authored-by: marioevz <11726710+marioevz@users.noreply.github.com>
Co-authored-by: marioevz <11726710+marioevz@users.noreply.github.com>
9a978a8 to
b95369f
Compare
Co-authored-by: marioevz <11726710+marioevz@users.noreply.github.com>
| # Storage is cleared for the first time in the transaction | ||
| # Issue refund for clearing a slot that was non-zero at tx start. |
There was a problem hiding this comment.
This'll need to be backported.
| ) | ||
| current_value = get_storage(tx_state, evm.message.current_target, key) | ||
|
|
||
| state_gas_storage_set = StateGasCosts.STORAGE_SET |
| gas_cost += ( | ||
| GasCosts.COLD_STORAGE_WRITE - GasCosts.COLD_STORAGE_ACCESS | ||
| ) | ||
| state_gas = state_gas_storage_set |
There was a problem hiding this comment.
| state_gas = state_gas_storage_set | |
| state_gas = StateGasCosts.STORAGE_SET |
| code_hash = get_account(tx_state, code_address).code_hash | ||
| code = get_code(tx_state, code_hash) | ||
|
|
||
| charge_gas(evm, extra_gas + extend_memory.cost) |
There was a problem hiding this comment.
Is that a behaviour change in this EIP? Something feels weird here.
In any case, if we can avoid committing incorrect specifications to forks/amsterdam, we should. I'm usually okay with deferring cleanup and backports to future pull requests, but this one goes beyond that IMO.
| memory_cost=Uint(0), | ||
| extra_gas=Uint(0), |
There was a problem hiding this comment.
This'll have to be backported then.
| class StateGasCosts: | ||
| """ | ||
| EIP-8037 state-gas constants. | ||
|
|
||
| Kept separate from `GasCosts` because these carry a different unit: | ||
| state-byte counts that convert into gas via `COST_PER_STATE_BYTE`. | ||
| Like `GasCosts`, these may be patched at runtime by a future gas | ||
| repricing utility to fast-iterate on state-byte costs. | ||
| """ |
There was a problem hiding this comment.
| class StateGasCosts: | |
| """ | |
| EIP-8037 state-gas constants. | |
| Kept separate from `GasCosts` because these carry a different unit: | |
| state-byte counts that convert into gas via `COST_PER_STATE_BYTE`. | |
| Like `GasCosts`, these may be patched at runtime by a future gas | |
| repricing utility to fast-iterate on state-byte costs. | |
| """ | |
| # These may be patched at runtime by a future gas repricing utility to fast-iterate on state-byte costs. | |
| class StateGasCosts: | |
| """ | |
| EIP-8037 state-gas constants. | |
| Kept separate from `GasCosts` because these carry a different unit: | |
| state-byte counts that convert into gas via `COST_PER_STATE_BYTE`. | |
| """ |
Let's keep the docstrings clear of implementation details.
| gas_left=Uint(0), | ||
| state_gas_left=message.state_gas_reservoir, | ||
| refund_counter=U256(0), | ||
| logs=tuple(), | ||
| accounts_to_delete=set(), | ||
| error=AddressCollision(), | ||
| return_data=Bytes(b""), | ||
| regular_gas_used=message.gas, | ||
| state_gas_used=0, | ||
| state_refund=Uint(0), |
There was a problem hiding this comment.
Changing from args to kwargs is what will need to be backported, or else the diff won't be readable.
Co-authored-by: kclowes <6540608+kclowes@users.noreply.github.com>
Co-authored-by: kclowes <6540608+kclowes@users.noreply.github.com>
Co-authored-by: kclowes <6540608+kclowes@users.noreply.github.com>
forks/amsterdam
…-by-design tests (ethereum#2843) * feat(forks): add Fork.oog_budget_lift helper for EIP-8037 OoG tests OoG-by-design ported_static tests are calibrated to land just below a cost boundary; on Amsterdam each fresh SSTORE-set and CREATE spills its state-gas portion back into regular gas when the per-tx reservoir is empty. Tests that expect "N SSTOREs and M CREATEs complete before OoG" need their gas budget lifted by N * sstore_state_gas + M * create_state_gas to land at the same intermediate state. `Fork.oog_budget_lift(sstores_before_oog=N, creates_before_oog=M)` composes the two existing state-gas helpers and returns 0 on pre-EIP-8037 forks (where both helpers are 0), so callers can apply it unconditionally without a fork guard. Unit-tested on Cancun (zero) and Amsterdam (cumulative spill). * fix(ported_static): Approach-1 fork-conditional OoG lift for stCreate*/stRevert*/stWallet* tests Eight tests share the OoG-by-design shape `tx_gas = [oog_path, success_path]` where the success_path budget is tuned to barely complete a single CREATE, a CREATE plus a few SSTOREs, or a deploy chain. On Amsterdam EIP-8037 splits NEW_ACCOUNT, fresh SSTORE-set, and code-deposit cost into a regular portion plus a state-gas portion; with an empty reservoir, the state-gas spills back into regular gas and breaks the success_path budget. Apply `Fork.oog_budget_lift` with the right (creates, sstores, deploy_code_size) counts to lift the budget on Amsterdam only. Pre- EIP-8037 forks return 0 from the helper, so the original budget is preserved. Files (skip-list entries cleared): - stCreate2/test_create2_oo_gafter_init_code.py (-g1) - stCreate2/test_create2_oo_gafter_init_code_returndata2.py (-g1) - stCreateTest/test_create_oo_gafter_init_code.py (-g1) - stCreateTest/test_create_oo_gafter_init_code_returndata2.py (-g1) - stRevertTest/test_revert_sub_call_storage_oog.py (-g1-v0) - stRevertTest/test_revert_sub_call_storage_oog2.py (-g1-v0) - stWalletTest/test_wallet_construction_oog.py (-g1) - stWalletTest/test_multi_owned_construction_not_enough_gas_partial.py (-g1) Removes 8 entries from amsterdam_skip_list.txt. * chore(ported_static): remove stale Amsterdam skip entries for stSStoreTest 16-pair family 22 stSStoreTest files (`sstore_0to*`, `sstore_xto*` excluding `gas`, `gas_left`, `change_from_external_call_in_init_code`) had 3 skip entries each (`d{0,1,2}-g1`). Re-running these on Amsterdam with the skip list disabled shows all 60 fixture variants per file already pass without any code changes. The post-state expectation at `g=1` is `contract_2: storage={1: 0}` (only slot 1, asserted to be zero). On Cancun, ~14 of the 16 SSTORE pairs complete before OoG; on Amsterdam EIP-8037 the state- gas spill cuts that to ~5 pairs. In both cases slot 1 ends at 0 and the final `SSTORE(1, 1)` does not run, so the assertion holds on both forks unchanged. These were defensive skip entries from an earlier snapshot. Remove all 66 entries; no test-file changes needed. * fix(ported_static): lift tx_gas[0] on Amsterdam for the 2 base oo_gafter_init_code tests Address kclowes's review on ethereum#2843: with only tx_gas[1] lifted, g=0 OoG'd at CREATE/CREATE2 dispatch on Amsterdam (NEW_ACCOUNT state-gas spill) before init code ever ran — the assertion still held (`NONEXISTENT` either way) but the failure mode shifted from "OoG after init code" (the test's named scenario) to "dispatch-time OoG". A clean closed form using `fork.oog_budget_lift(creates_before_oog=1)` (183600) overshoots and pushes g=0 past the deploy threshold. The Cancun 1000-gas gap between g=0 and g=1 collapses on Amsterdam: once dispatch is cleared, the 5-byte init code is cheap enough to always complete. Empirical binary-search on both files puts the safe range at (166499, 167000); 166_750 sits in the middle, keeping g=0 OoG'ing at dispatch and g=1 just clearing the deploy threshold. The two `_returndata2` variants are left unchanged — g=0's post-state happens to land identically on both forks at the existing budget, and adding any lift breaks them. * fix(spec-specs, test-types): resolve lint errors from forks/amsterdam → devnets/bal/7 merge The May-18 merge `dffc4cfea` ("Merge remote-tracking branch 'upstream/forks/amsterdam' into devnets/bal/7") had two conflict resolutions that left `bal/7` in a state where `just static` fails: 1. `src/ethereum/forks/amsterdam/blocks.py` ended up with the EIP-7843 `slot_number: U64` field declared twice (lines 260 and 268). `mypy` rejects it with `[no-redef]`; `ethereum-spec-lint` crashes with `ValueError: duplicate path Header.slot_number`. Remove the second copy. 2. `BuiltBlock.derive_engine_payload_modifier` was dropped from `packages/testing/src/execution_testing/specs/blockchain.py` while its 5 call sites in `specs/tests/test_types.py` were kept. `mypy` reports 5 `attr-defined` errors. Restore the staticmethod (and the `FixtureExecutionPayloadModifier` import it needs) from `forks/amsterdam`. The instance-level wiring on `forks/amsterdam` (`BuiltBlock.rlp_modifier` field, constructor pass-through, and `get_fixture_engine_new_payload` call) is **not** restored — neither the linter nor the test file references it, and `bal/7`'s current `get_fixture_engine_new_payload` already runs without it. This unblocks CI on every open PR against `devnets/bal/7` (including this one).
|
One observation on the Amsterdam gas bumps — non-blocking, more of a The fork-conditional bumps across 1. Flat 2. execution-specs/tests/ported_static/stCallCodes/test_callcall_00.py Lines 49 to 52 in 9a978a8 3. Programmatic, derived from the cost model — the bump is computed from the tx_gas[0] = 200000
if fork.is_eip_enabled(8037):
tx_gas[0] += 4 * Op.SSTORE(new_value=1).state_cost(fork)execution-specs/tests/ported_static/stStaticCall/test_static_call10.py Lines 173 to 174 in 9a978a8 execution-specs/tests/ported_static/stEIP2930/test_storage_costs.py Lines 665 to 673 in 9a978a8
execution-specs/packages/testing/src/execution_testing/vm/bytecode.py Lines 311 to 322 in 9a978a8 I'd lean toward form 3 as the target. The hardcoded numbers in forms 1 and 2 This doesn't need to hold up the PR — form 3 already exists in the tree, so the |
🗒️ Description
This PR merges EIP-8037 into
forks/amsterdamincluding framework, specs and all test changes.Review Plan
Specs -
src/ethereum/forks/amsterdam/@SamWilsnTests batch 1 - Targeted 8037 cases,
tests/amsterdam/, 54 files @marioevzTests batch 2 - Cross fork non-static,
tests/osakatotests/frontier, 78 filesTests batch 3 - Cross fork static-ported,
tests/ported_staticaudit @leolaraTesting Framework -
packages/testing@marioevz🔗 Related Issues or PRs
N/A.
✅ Checklist
just statictype(scope):.